The latest updates on your projects. Learn more about Vercel for Git ↗︎

Hey, @vonagam, I think I missed this diff. Sorry. Thank you for making this PR

Here's the issue I was trying to resolve using void in Maybe:

Basically, TS treats undefined and void differently in function returns. Didn't dig into it, but removing void from Maybe makes it so that tests without a return value will fail the compiler.

An alternative would be to add void directly to TestResult, but that's pretty much the same thing, isn't it? Maybe there's a smarter way that I am not aware of.

Also, I am not fully sure what would cause the issue you're seeing. Can you put a repro on codesandbox and I'll try to look at it? Wondering if it is a pnpm thing, vest thing, or something completely different.

Tried a quick repro on CodeSandbox and I didn't experiecne it, so interesting to see why it is behaving like that.

About repo: I have not used codesandbox much but this is part of an example that produces the error:

export class Wrapper {
  //...
  getError() { // <- mentioned error will be there
    return this.suiteResult.getError();
  }
}

Need to have "declaration": true in tsconfig as this one about reexporting types.

About no return: No return functions can be assigned to type of functions with undefined return without a problem - playground. But void functions (so typed as void) cannot be assigned to anything besides void functions - playground. So what you shown can be made work without an error but something prevents it. Edit: Intersting, () => {} can be assigned to () => undefined but not to () => undefined | boolean, feels like a bug in typescript.

Annoying, so in microsoft/TypeScript#36288 ts got ability to accept () => {} as () => undefined but not as () => undefined | T which I find inconsistent... There is also microsoft/TypeScript#36239 which will solve the issue.

As for now, removed modification to Maybe and just purged its usage from selectors types.

For record, seems like TestResult is the only time when void is needed in Maybe, so it is still possible to remove void from Maybe and just define TestResult as Maybe<...> | void.

Ok, two things.

First, published my integration of vest for svelte. So now it should be possible to see the errors I am talking about. Let's take this code:

getError(): Suite.Failure<V, A> | undefined {
  return this.summaryStore.value.getError() as any;
}

Ideally I would want not to specify types and let everything be inferred:

getError() {
  return this.summaryStore.value.getError();
}

But if I do that and run pnpm tsc I currently get:

The inferred type of 'getError' cannot be named without a reference to '.pnpm/[email protected]/node_modules/vest-utils'. This is likely not portable. A type annotation is necessary.

So I am required to specify a type, but if I do it like this:

getError(): Suite.Failure<V, A> | undefined {
  return this.summaryStore.value.getError();
}

Then I get an error that says:

Type 'void' is not assignable to type 'SummaryFailure<Field, string> | undefined'.

That's the problem I am trying to solve.

Second, so I've reverted PR to almost initial state where I removed void from Maybe (as title says), but, as you pointed out, changed TestResult type to be Maybe<> | void to match that use case (that's the only place where Maybe is used as a public return type for that purpose).

Thoughts?

This seems reasonable. I created Maybe as a very simple helper utility type to help me cope with having to type things repeatedly, but OTOH, maybe that's not the best option when exposing a publicly facing API, and the best approach is to be explicit to begin with.

Let me just test this locally real quick, and if this works, I'll merge, and release a @next version just so we can test it. If everything works then, we can release a patch version early next week.

Also, @vonagam, the vest-for-svelte repo looks great! Thank you so much for working on it
May I add it to the community integrations section of the website?

https://vestjs.dev/docs/community_resources/integrations

Thanks. Yes, of course, have nothing against the library being listed there.

OK, @vonagam,
vest@next can already be install. The exact tag is: [email protected].

I'll add the resource to the website tomorrow, I'm after a very long international flight, so it will take me just a little bit more time. Thank you for all the effort you're putting into Vest!

Ok, with that version I can remove as any from getError/getWarning usages. Which is good.

But seems like I still need to specify return type because Maybe is used in SummaryFailure definition. If it is replaced there with simple | undefined then errors go away.

Can I add this change? (Checking because you made a release, don't want to mess things up.)

@vonagam, yes, sure. Go ahead. The release goes through a separate branch, no issues there.

@vonagam - OK, @next now is [email protected]. Let's try this one.
I'll head out for today, but if that works for you, I'll get that merged and published soon.

Yep, works without a problem.

@vonagam, released as [email protected]